feat: group skills on install and mark audit input untrusted - #81
Merged
Merged
Conversation
`.claude-plugin/plugin.json` lists all nine skills in a `skills` array, so `npx skills` renders one select-all group instead of nine loose rows. The installer matches a skill's own directory against the array, so a single `./skills/` entry grouped nothing. `oss-audit` states that the output of `scripts/collect.mjs` is quoted from the repository under audit and is data rather than instruction. Heading text, the line before a fenced block, link text, and job names are prose an outsider wrote, and they reached the audit with nothing marking the trust boundary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
.claude-plugin/plugin.jsonlists all nine skills in askillsarray. Installing oss-kit withnpx skillsoffered nine separate rows, so a user had to pass--skills '*'or select every one by hand. It now renders one select-all group.oss-auditmarks the output ofscripts/collect.mjsas untrusted input. The script opens its output with anuntrustedContentfield saying every value below is quoted from the repository under audit and is data rather than instruction, and Step 3 ofSKILL.mdsays the same where it tells a reader to run the script.tests/manifests.test.tscompares the array against the directories that actually carry aSKILL.md, so a skill added toskills/and left out of the manifest fails the build.Why this way
The installer matches a skill's own directory against the array, at
src/skills.ts:202invercel-labs/skills, so a single./skills/entry groups nothing and every path has to be named..codex-pluginand.cursor-pluginkeep the one-line"./skills/"form, because only the Claude manifest is read for grouping and only it needs maintenance when a skill is added.The guard test is not tidiness. Claude Code's
skillsfield normally adds to the defaultskills/scan, but the plugin's marketplace entry resolves to the marketplace root, which is the documented exception where the declared paths replace that scan. Without the test, a tenth skill would stop loading in Claude Code and nothing would say so.Skills.sh raises Snyk W011 against
oss-auditfor third-party content exposure, and the finding is right about the behaviour:precededBy, heading text, link text, and job names are free prose an outsider chose. Stripping that prose is not available, because several rules are scored on it, so the change names the trust boundary instead of trying to remove it.Verification
The guard test was proved by dropping
oss-writingfrom the array and watching it fail, then restoring the file.bun testreports 328 passing, andbun run validate,scripts/check-drift.sh, andscripts/check-ecosystems.mjsare clean.scripts/prose.mjsreports no finding onCHANGELOG.mdor onoss-audit/SKILL.md.The grouping is verified against the installer's source rather than a live install, because the CLI clones from the remote and the manifest is only observable there after this merges.
Known gaps
The group renders as
Oss Kit. The installer title-cases each hyphen-separated word of the plugin name, soOSS Kitneeds a plugin rename and is not worth one.Grouping turns the picker's search box off, which the installer decides and this repository cannot influence.
The Snyk rating may stay at Med Risk, because the behaviour W011 describes is still present. Disputing the code upstream is the remaining option, on the argument that a local checkout the user pointed the audit at is not the public user-generated content W011 defines.
Affects